Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Math.Tau, MathF.Tau #37517

Merged
merged 2 commits into from
Jun 7, 2020
Merged

Add Math.Tau, MathF.Tau #37517

merged 2 commits into from
Jun 7, 2020

Conversation

john-h-k
Copy link
Contributor

@john-h-k john-h-k commented Jun 5, 2020

Added Math.Tau and MathF.Tau constants. Tests for Math/F.E and Math/F.PI were also introduced to check they are the expected value for consistency 😄

Fixes #24678

@Dotnet-GitSync-Bot
Copy link
Collaborator

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

@john-h-k
Copy link
Contributor Author

john-h-k commented Jun 5, 2020

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public > APIs, to please make sure the API implementation in the src *.cs file is documented with > triple slash comments, so the PR reviewers can sign off that change.

Neither E nor PI have triple slash docs so for consistency I stayed that way, but I see that Tau is a less well-known constant so they may be useful

@john-h-k
Copy link
Contributor Author

john-h-k commented Jun 5, 2020

cc @tannergooding

tannergooding
tannergooding reviewed Jun 7, 2020
View reviewed changes
src/libraries/System.Runtime/ref/System.Runtime.cs
@@ -2532,6 +2532,7 @@ public static partial class Math
{
public const double E = 2.718281828459045;
public const double PI = 3.141592653589793;
public const double Tau = 6.283185307179586;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting for anyone that comes looking, the value we put in the src is the actual value of PI to at least 17 significant digits + 1 rounding digit (or to at least 9 significant digits + 1 rounding digit for float).

The value here is the actual double or float value (which may be different due to rounding as the actual value may not be exactly representable) formatted as a string with at least enough precision to correctly roundtrip. Hence why the ref float versions differ slightly from the "true digits" of PI or TAU

REF FLT: 3.1415927
SRC FLT: 3.1415926 5
REF DBL: 3.1415926 53589793
SRC DBL: 3.1415926 53589793 23846
TRU PI:  3.1415926 53589793 23846 2643383279502884197169399375105820974...

REF FLT: 6.2831855
SRC FLT: 6.2831853 07
REF DBL: 6.2831853 07179586
SRC DBL: 6.2831853 07179586 476925
TRU TAU: 6.2831853 07179586 476925 286766559005768394338798750211641949...

@tannergooding tannergooding merged commit 16e7c69 into dotnet:master Jun 7, 2020
@john-h-k john-h-k deleted the add-tau branch June 7, 2020 16:18
@aaronfranke
Copy link

Should this be placed on the 5.0 milestone, since I assume it will be available in .NET 5?

@tannergooding tannergooding added this to the 5.0 milestone Jun 7, 2020
@Goutte Goutte mentioned this pull request Aug 10, 2020
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tannergooding tannergooding tannergooding approved these changes

Assignees
No one assigned
Labels
area-System.Runtime new-api-needs-documentation
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.

API Proposal: Add Tau to System.Math(F) class
4 participants
@john-h-k @Dotnet-GitSync-Bot @aaronfranke @tannergooding